Engine: stop waiting on a launcher window that does not answer - #265
Merged
Merged
Conversation
forward() held the window slot for as long as the window took to answer a pushed command. A wedged window therefore blocked every request that needs it (toggle, show, HUDs, dmenu, deeplinks) until the daemon restarted; only the CLI had a timeout. forward() now gives the window 5 seconds, under the CLI's 10, and answers with an error naming the wait. The link is kept: the window may only be busy, and dropping it would leave the launcher undriven until it restarts. The window still answers the abandoned command in order, so WindowLink::push now skips answers older than its own command rather than failing on the mismatched id. Tests: a dropped push does not steal the next push's answer (compass-ipc, fails without the skip), and forward_within times out on a slow window, keeps it, and reads the next answer once it catches up. Part of #257. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtGVEzDmYTdpsuEQErmmLn
The wlroots track failed on the first Show of two tests: on a runner without a GPU the new window's renderer took about five seconds to come up, and the engine gave up on a launcher that was still starting. The limit is there to bound a stuck window, not a slow one, so it is now longer than the CLI's 10 seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtGVEzDmYTdpsuEQErmmLn
![hanthor-hive-agent[bot]](https://avatars.githubusercontent.com/in/3942065?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks correct to me. forward_within bounds the slot lock and returns an actionable error on timeout (crates/compass/src/serve.rs:2572-2589) without discarding a live window link. The correlation loop then consumes only older late replies before requiring the new command's exact ID (crates/compass-ipc/src/transport.rs:302-328), matching the serialized bridge and the added end-to-end coverage.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
6 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the window-slot item in #257.
Problem
serve::forwardlocks the window slot and awaitsWindowLink::pushwith no time limit. If a launcher window is alive but wedged, every request that needs the window (toggle, show, hide, HUDs, dmenu, deeplinks, launches) waits on that lock until the daemon restarts. Only the CLI had a timeout (10 s).Change
crates/compass/src/serve.rs):forwardnow goes throughforward_withinwithWINDOW_ANSWER_TIMEOUT= 30 s. On timeout it answers with an error that names the wait.Showwaits on its renderer, and on a machine without a GPU that takes seconds; it took about 5 s on the wlroots CI runner. The first push used 5 s and failed two Sway tests for exactly this reason.crates/compass-ipc/src/transport.rs): a window still answers the command the engine gave up on, in order and before the next one.WindowLink::pushnow skips answers older than its own command id, instead of failing withMismatchedResponseand getting the window dropped. The new# Cancellationdoc section explains this. Framed I/O is cancel-safe, so an abandoned write is finished by the next send.Tests
compass-ipc,a_push_given_up_on_does_not_steal_the_next_ones_answer: the first push is dropped after 100 ms, the window then answers it late, and the next push reads its own answer. This test fails with the skip removed (control run).compass,window::tests::the_engine_stops_waiting_on_a_slow_window_and_keeps_it: runs through the real bridge.forward_withintimes out with an error, the slot still holds the link, and after the UI catches up,forward(Hide)is acknowledged.wlroots_engineon local headless Sway: all 16 pass with 30 s. With the limit forced to 200 ms, the two tests that failed in CI fail the same way, which confirms the cause.cargo test -p compass-ipc -p compasspasses in full;cargo clippy -p compass-ipc -p compass --all-targets -D warningsis clean;cargo docwith-D warningsis clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01UtGVEzDmYTdpsuEQErmmLn